Skip to content
This repository was archived by the owner on Jan 19, 2023. It is now read-only.

Adding an attribute to skip the margin offset for the fab#27

Open
ericmartineau wants to merge 17 commits intomarianocordoba:masterfrom
SunnyApp:master
Open

Adding an attribute to skip the margin offset for the fab#27
ericmartineau wants to merge 17 commits intomarianocordoba:masterfrom
SunnyApp:master

Conversation

@ericmartineau
Copy link
Copy Markdown

In my case, I was positioning the FAB using a stack (I'm using ios scaffold widgets).

The margin offsets were causing my button to float slightly to the left and downward. This attribute allows for removing the margin offset transformation.

@marianocordoba
Copy link
Copy Markdown
Owner

Hi @ericmartineau. Thank you for your PR!
Can you please update the README with the documentation for the new property?

Copy link
Copy Markdown
Contributor

@MrCsabaToth MrCsabaToth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not the author but I think these should be discussed before any merging.

{Key key,
this.alignment = Alignment.bottomRight,
this.buttonKey,
this.alignment = Alignment.bottomCenter,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't change the default alignment, that would break existing app UXs

// textDirection: textDirection ?? Directionality.of(context),
// fit: fit,
// overflow: overflow,
// );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented code

import 'package:flutter/rendering.dart';

/// Passes all events to all children of the stack. The FAB was having issues
/// where the padding on the button was blocking the circular items on the edge
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is for #14 then it's not the right fix.


// FAB
Container(
margin: widget.fabMargin,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be faulty here? The Container's margin is removed earlier along with the compensatory translation for that margin. If we add a margin here then it margins the button like originally it wasn't intended to. Correct me if I'm wrong. I find it very good idea to simplify the widget stack by removing the original margin and the translation.

@MrCsabaToth
Copy link
Copy Markdown
Contributor

It'd be so great if we could just ditch the

  Container(
      margin: widget.fabMargin,
      // Removes the default FAB margin
      transform: Matrix4.translationValues(
        16.0 * _directionX,
        16.0 * _directionY,
        0.0,
      ),
      child:

and just start the build return with the Stack. It works for me,

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants